feat: Replace terminal dangerous-command block with HITL approval and add Safe Mode in Settings#1310
Conversation
|
@bytecii @Wendong-Fan Could you please review my PR? |
bytecii
left a comment
There was a problem hiding this comment.
IMO we can move this to camel to make a special safe mode such as strict. cc @Wendong-Fan
thanks @bytecii , the idea behind this issue is to implement a human-in-the-loop mechanism for safer code execution, which would involve the HumanToolkit. Since CAMEL serves more as a modular framework, it might be more appropriate to implement this feature in Eigent as an application layer. What do you think? |
|
@Wendong-Fan @bytecii could you please review this pr? Regards |
- Move HitlOptions and ApprovalRequest models from chat.py to app/hitl/__init__.py - Remove unnecessary hasattr guard on auto_approve (always initialized) - Remove issue reference comment and docstring comments - Update all imports across controllers, services, and tests
- Move _request_user_approval from AbstractToolkit to TerminalToolkit (only terminal uses approval today) - Remove debug print statements from shell_exec - Move import time to top-level in terminal_toolkit - Fix approval endpoint docstring - Remove module docstring from terminal_command.py - Add comment for effective executable check - Update test to subclass TerminalToolkit directly
| options.project_id, | ||
| Agents.browser_agent, | ||
| working_directory=working_directory, | ||
| safe_mode=True, |
There was a problem hiding this comment.
remove this as this is not configured. And by default if the HITL not specified, we still use the safe_mode=True for the terminaltoolkit
| return None | ||
| return "Operation rejected by user. The task is being stopped." | ||
|
|
||
| async def shell_exec( |
- Remove unused logger from abstract_toolkit - Move _thread_local from AbstractToolkit to TerminalToolkit - Add missing threading.local() definition - Add Args/Returns to terminal_command.py functions - Use ApprovalAction.reject enum instead of raw string - Fix ApprovalAction and ActionCommandApprovalData docstrings - Add comment for auto_approve reset at task start
|
@fengju0213 @Wendong-Fan @bytecii I updated all the details by following the feedback, ready for review. :) |
|
could you please check @fengju0213 ? Hope we can complete this task ASAP... |
|
Could you let me know your plan, @fengju0213 ? |
|
Still waiting for review, @Wendong-Fan @bytecii.. Regards |
|
@fengju0213 Could you please review current PR? or hope you can let me know about any timeline for this, remaining for review status for several weeks.. Regards. |
Sorry for the delay. Our team has been extremely busy over the past few weeks. On the code side, we don't have any major issues at the moment. However, since this involves UI and user experience, we need our product team to review the UI part. I will ask them to provide feedback as soon as possible. Once again, apologies for the delay! |
|
@bytecii Could you please review this PR? |
Related issue
Closes #1306
Summary
Replaces the Terminal Toolkit’s hard block of “dangerous” commands with a Human-in-the-Loop (HITL) approval flow and adds a Safe Mode setting so users can opt in.
Changes
HITL for dangerous commands
/chat/{id}/terminal-approvalwith the chosen option; backend enforces approval before running the command and supports “approve all in task” per task.Safe Mode in Settings
“With Safe Mode active, Eigent will pause and seek explicit approval whenever high-risk system operations are detected.”
Backend
sudo,su,reboot,shutdown), file (e.g.rm,chown,mount), disk (e.g.dd,mkfs,fdisk), process (e.g.service,systemctl), network (e.g.iptables,ifconfig), cron (e.g.crontab,at), user/kernel (e.g.useradd,modprobe), and related commands as specified.cdis validated so the agent cannot leave the designatedworking_directory.approved_all_dangerous_commandsand a queue for terminal approval; reset on new task.Frontend
localStorageand sent assafe_modein the start-task request.Other
uvis not installed so commits succeed withoutuv(message suggests installing uv for backend checks).Testing
cdoutsideworking_directoryin non-Docker mode is rejected.